Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Remove Explore URLs and make the normal URLs public #2632

Merged

Conversation

michael-genson
Copy link
Collaborator

@michael-genson michael-genson commented Oct 14, 2023

What type of PR is this?

(REQUIRED)

  • bug
  • cleanup
  • feature

What this PR does / why we need it:

(REQUIRED)

This removes the frontend explore pages and adapts the "private" pages to allow public access (unless things are marked as private, of course).

To accomplish this, all pages (with a few exceptions, see below) now have the group slug in their URL. So, for example:

  • This URL is invalid: https://my-mealie-domain.com/recipe/pasta-fagioli
  • This URL is valid: https://my-mealie-domain.com/home/recipe/pasta-fagioli

I'm not too horrible of a person, so any time you reach the 404 page and you're logged in, we try to redirect you to .../${groupSlug}/${whereverYouWereTryingToGo} in an attempt to preserve existing links. This usually works, though there are bound to be edgecases. In the above example, for instance, the invalid URL will redirect you to the valid URL (if you're logged-in, otherwise we don't know which group to put you in).

The only non-group-slug-dependent pages are the following:

  • all admin pages (since you manage app-wide things there)
  • the registration page (because there was no need to change it)
  • forgot-password, login, and reset-password pages (since we'll know where to put you once you log in)
  • the main index page (although it will re-direct you immediately based on your logged-in state)

As a side-effect, all of the public URL copying (on recipes and on your user profile page) has been removed, since it's useless now. The recipe share functionality is still there, though, since that allows private access.

While I'm already messing with the sidebar, I noticed the cookbook secondary header wasn't working (hence #2679) so I got that working again. I also conditionally hid it if you don't have any cookbooks yet.


As an added bonus to this already-too-big-for-my-own-good PR, I tweaked the loggedIn refs throughout the app to check if the logged-in user belongs to the group. This way a logged-in user can stay logged-in while visiting other groups (you can't really do anything special, you just aren't forced to log out to view other groups now).

This leads to some weird scenarios if you try manually adjusting your URL (e.g. if you go to .../not-my-group-slug/shopping-lists you will be successful, but it's still going to show you your group's shopping lists). Fixing that would probably take 10x more work than the work done on this PR, since the app was originally built with the assumption that you're always in your own group, but there is no way through the UI to get to this weird state (that I know of, anyway), which I think is adequate.

Another option is to instead keep user and group pages within their own spaces <-- I did this. I moved cookbooks and the recipe timeline under .../${groupSlug}/recipe/... to accommodate this.

Which issue(s) this PR fixes:

(REQUIRED)

Closes discussion #2679

Special notes for your reviewer:

(fill-in or delete this section)

Tons of things were changed to get this to work, but it's not too much of a mess. Thankfully all the lower-level stuff didn't need much/any modification. I had to do some funky stuff with the routes/layouts to inject your group slug when you log in (since the login page is group-agnostic). It probably needs a few eyes on it to make sure nothing too jank is happening.

Testing

(fill-in or delete this section)

Unfortunately, this PR touches pretty much every page and most of the main components, and none of it affects anything with test coverage, so all testing is manual. I did a lot of poking around and global searching in VSC for patterns likely to break, but there are bound to be edge cases.

Release Notes

(REQUIRED)

migrated all pages to include the group name in the url
migrated all publicly-accessible pages to use the same urls as the private pages (if public access is enabled)
moved cookbook and timeline page URLs
users can stay logged-in while visiting other groups, but are treated as a "public" user
removed all explore pages

@michael-genson
Copy link
Collaborator Author

michael-genson commented Oct 29, 2023

I changed the routes as discussed above (I went with /r/ to replace /recipes/ since we're kinda doing the same thing with groups and /g/). I need to play around with it more just to confirm nothing broke, but it looks good. I also updated the 404 logic to migrate /recipe/... links to /r/...

I realized the metadata injection was broken in prod since the FastAPI SPA was manually constructing those routes. I'm pretty sure I fixed it, but I need to verify with a prod docker build (hopefully I'll have time later tonight, if not tomorrow, unless you get the chance to confirm)

@michael-genson
Copy link
Collaborator Author

Not sure if I'm just doing something wrong or I broke something but running the docker compose locally doesn't work. I'm getting: No such file or directory: '/spa/static/index.html' (which shouldn't happen since that file should be there)

Copy link
Collaborator

@hay-kot hay-kot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pushed a fix for the build issue. I confirmed the sudo SSR is working. Can you double check the SSR and then merge?

Super stoked to push this one out 🚀

@michael-genson michael-genson enabled auto-merge (squash) November 5, 2023 21:03
@hay-kot hay-kot self-requested a review November 6, 2023 01:06
@hay-kot hay-kot disabled auto-merge November 6, 2023 01:06
@hay-kot hay-kot merged commit 80968b0 into mealie-recipes:mealie-next Nov 6, 2023
@michael-genson michael-genson deleted the feat/merge-explore-urls branch November 6, 2023 01:47
@dathbe
Copy link

dathbe commented Nov 29, 2023

So what does this now make the public url for viewing all recipes? I had a shortcut on my iPhone for that page, which no longer works, and it seems this is the reason. Is there still a link to view all recipes, or does public access only work for specific recipes like the example above?

On a somewhat related note, the old link was a redirect, and the redirected-to url was not publicly accessible directly, so I had to take the janky step of turning off my internet connectivity to save the public url to my iPhone home screen. Is that something that has changed with this update? If not, I can submit a feature request.

I find that it's nice to be able to share your catalogue with family and friends without requiring them to log in. But more selfishly, it's just far easier to have the public read-only link quickly accessible from my phone for those times when your hands are covered in food.

@michael-genson
Copy link
Collaborator Author

So what does this now make the public url for viewing all recipes?

The same URL you use to browse while logged-in. There's no special URL anymore. So, if you navigate to your list of recipes, copy that link, log out, and try going to that link again. You'll see all your recipes, but you won't be logged-in.

@dathbe
Copy link

dathbe commented Nov 29, 2023

Thanks. I can confirm that using https://myurl.com/g/home or https://myurl.com/g/home?orderBy=created_at or https://myurl.com/g/home?orderBy=name&orderDirection=asc and various other combinations does indeed allow you to view the recipes without logging in.

It's still creating an issue with iOS shortcuts, though, because when I try to add that page to my home screen it instead creates a link to https://myurl.com, which just redirects to the login screen. I assume this is some quirk of how iOS/mobile Safari deal with creating shortcuts and not a problem per se with Mealie, but I wanted to flag it because it is probably something we can create an easy workaround to. The trick of creating the shortcut while offline isn't working now.

@nkringle
Copy link
Contributor

Thanks. I can confirm that using https://myurl.com/g/home or https://myurl.com/g/home?orderBy=created_at or https://myurl.com/g/home?orderBy=name&orderDirection=asc and various other combinations does indeed allow you to view the recipes without logging in.

It's still creating an issue with iOS shortcuts, though, because when I try to add that page to my home screen it instead creates a link to https://myurl.com, which just redirects to the login screen. I assume this is some quirk of how iOS/mobile Safari deal with creating shortcuts and not a problem per se with Mealie, but I wanted to flag it because it is probably something we can create an easy workaround to. The trick of creating the shortcut while offline isn't working now.

There is a workaround that you could use today by creating a redirect rule if you have mealie behind a load balancer/reverse proxy. See the POC on this feature request.

@michael-genson
Copy link
Collaborator Author

when I try to add that page to my home screen it instead creates a link to https://myurl.com

I'm not familiar with iOS shortcuts, but yeah, that seems weird

There is a workaround that you could use today by creating a redirect rule if you have mealie behind a load balancer/reverse proxy

FYI I have a PR in for this now: #2772

@boc-the-git
Copy link
Collaborator

Re the iOS shortcuts issue, can you provide specific detail for how that is being setup? The ideal would be a screen recording of start to finish. That might allow us to understand it better.

(I'm wondering if it might be using the PWA or something)

@dathbe
Copy link

dathbe commented Nov 29, 2023

Sure.

  1. Open mobile Safari and navigate to https://myurl.com/g/home
    2023-11-29 12-40-58

  2. Page will load as expected and user will be logged out.
    2023-11-29 12-40-16

  3. Click the share icon at the bottom middle of the screen to bring up the share menu.
    2023-11-29 12-40-34

  4. Click "Add to Home Screen," which brings up the shortcut dialogue but, curiously, only includes the root domain without the necessary "g/home". (In Apple's wisdom of always trying to dumb things down, the url is not editable.)
    2023-11-29 12-40-41

@boc-the-git
Copy link
Collaborator

Annoying you can't edit the url.

Are you comfortable building a Dev instance? I wonder if this is the cause of the url being what it is: https://github.com/mealie-recipes/mealie/blob/mealie-next/frontend/nuxt.config.js#L336

I'm not sure we realistically have much scope to change that though.

@michael-genson
Copy link
Collaborator Author

michael-genson commented Nov 29, 2023

I'm not sure we realistically have much scope to change that though.

#2772 should at least fix this for users only using a single group (or who want all public users to be directed to the default group). This would effectively make the "default group" also the "iOS Shortcut" group.

No idea if such a thing exists to have multiple start urls.

@dathbe
Copy link

dathbe commented Nov 29, 2023

Yeah, I don't know enough about iOS shortcuts to know how it works. But on RC1.1, you could get it to work using the offline method I described above. And yes, 2772 should do what I'm looking for. It's one extra click if you actually do want to log in, but that's fine for my purposes. I'll wait for it to get merged.

I'm sure I could figure it out, but I've never built a dev instance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants